Skip to content

test(suidhelper): security-gated test seam + parametrized E2E suite#26

Merged
markovejnovic merged 12 commits into
mainfrom
test/suid-e2e
Jun 25, 2026
Merged

test(suidhelper): security-gated test seam + parametrized E2E suite#26
markovejnovic merged 12 commits into
mainfrom
test/suid-e2e

Conversation

@markovejnovic

@markovejnovic markovejnovic commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

No description provided.

Route the config file path through security_gate::split so subprocess
tests can redirect it via HYPER_SETUIDHELPER_CONFIG_PATH — only when
both gates are open (insecure_test_seams feature + env var). Production
builds always read /etc/hyper/config.toml with full SafeFile checks.

Widened LoadingError path variants from &'static Path to PathBuf;
dropped the Copy derive and removed the now-unused CONFIG_PATH static.
Remove the inner security_gate::split around the SafeFile read in
Config::safe_load. The seam was incorrectly bypassing SafePath lexical
and SafeFile ownership/type/writability checks in insecure test mode.
Now only config_path() is gated — the path changes, but validation is
identical in both modes. Update config_seam test to prove the override
path is consulted via the real lexical gate: a .. component in the
override is rejected as LooseComponents, which cannot come from the
clean hardcoded default, confirming the seam without bypassing any
security check.
Add integration tests for the three security-critical operand parsers that
were previously untested. SafeBin tests cover all five refusal axes (relative
path, wrong basename, symlink, non-root owner, group/other-writable); root-
only axes self-skip when running unprivileged. DmTable tests assert the full
accept+round-trip contract (canonical forms, whitespace normalisation) and the
reject matrix (non-whitelisted targets, arbitrary devices, bad flags, wrong
arity). ThinMessage tests cover the two whitelisted verbs with normalisation
and the reject set (unknown verbs, missing/extra args, non-numeric id).

Also exposes DmTable and ThinMessage via pub use from the tools module so
integration tests can import them without reaching into internal submodules.
Adds tempfile = "3" dev-dependency for root-independent filesystem fixtures.
Add tests/e2e/config.rs (feature-gated on insecure_test_seams) that
drives the real binary via CARGO_BIN_EXE to assert exit codes and
stderr for missing, non-root-owned, malformed, relative, and valid
configs. Two cases always run as non-root; three skip with a loud
eprintln! and pass when not root. Register e2e_config [[test]] target.
Add tests/e2e/argv.rs (feature-gated insecure_test_seams) that points the
helper's --bin at a root-owned shell fake recording argv as JSON. Asserts
exact command lines for dmsetup create/remove/message and blockdev --getsz.
All four tests early-skip with a loud eprintln when not root; real assertions
run in the privileged CI job.
Add `suidhelper-privileged` job that runs only `--test e2e_config --test
e2e_argv` under `sudo -E` so the root-only cases (sys-test ok, fake-bin
argv capture) actually execute in CI. The non-root `rust` job continues
to run the full suite with `--all-features`; owner-axis tests that assume
a non-root uid stay there. Append a Testing section to the suidhelper
README documenting the three invocation tiers and the dual-gate insecure
test seam.
@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.25000% with 6 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
native/suidhelper/src/config.rs 66.66% 5 Missing ⚠️
native/suidhelper/src/main.rs 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown

Test Results

209 tests  +51   209 ✅ +51   2s ⏱️ -1s
 42 suites +12     0 💤 ± 0 
  2 files   ± 0     0 ❌ ± 0 

Results for commit e51803e. ± Comparison against base commit e81fa81.

♻️ This comment has been updated with latest results.

The INSECURE flag is written once at startup before any concurrency and
publishes no other memory, so SeqCst overstates the contract; Relaxed is
sufficient.
@markovejnovic markovejnovic merged commit b3c9781 into main Jun 25, 2026
6 checks passed
@markovejnovic markovejnovic deleted the test/suid-e2e branch June 25, 2026 03:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant